Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stevo]: shift register enable signal now enables ALL registers in th… #370

Merged
merged 3 commits into from
Nov 18, 2016

Conversation

stevobailey
Copy link
Contributor

…e chain.

Previously, the enable signal only worked on the first register. Later ones still shifted every cycle. Now they are all controlled by en.

This also fixes the problem in PR #355, so I'll close that one in favor of this one.

@aswaterman
Copy link
Member

shouldn't the enable be pipelined down, rather than simply fanning out to all of the stages?

@stevobailey
Copy link
Contributor Author

But then "disabling" the shift register just causes the value in the first register to shift down, filling all the registers, as is currently the case. When would this be the desired behavior?

My intended use of the enable signal is to enable or disable the shifting for a cycle.

@aswaterman
Copy link
Member

I see your point. I guess what I'm describing is Pipe, not ShiftRegister.

@davidbiancolin
Copy link
Contributor

Why not also remove the n == 1 case now that this recurses more regularly?

@aswaterman
Copy link
Member

Agreed.

@aswaterman aswaterman merged commit b2afa96 into chipsalliance:master Nov 18, 2016
@ccelio
Copy link
Contributor

ccelio commented Mar 2, 2017

This is an unacceptable change to the Chisel API. In the future please adhere to a deprecation schedule.

@aswaterman
Copy link
Member

In retrospect, I completely agree, @ccelio. Sorry about that.

mwachs5 pushed a commit that referenced this pull request Dec 29, 2022
Bumps [chiseltest](https://github.com/ucb-bar/chisel-testers2) from `5ede0ae` to `5e81249`.
- [Release notes](https://github.com/ucb-bar/chisel-testers2/releases)
- [Commits](ucb-bar/chiseltest@5ede0ae...5e81249)

---
updated-dependencies:
- dependency-name: chiseltest
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants